Read a materialized top-K by naming its entity and dimension values (Step 5 of #386) - #497
Open
zipdoki wants to merge 1 commit into
Open
Read a materialized top-K by naming its entity and dimension values (Step 5 of #386)#497zipdoki wants to merge 1 commit into
zipdoki wants to merge 1 commit into
Conversation
1 task
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rank rows are written and kept fresh, but nothing can read them. Getting a ranking out means knowing the rank table's name, its index and how its row key is composed, then scanning it yourself. This exposes the read.
The path names the source table and the declaration on it, never the rank table.
RankScan.from(...)reads the declaration off the schema and resolves the rest — which table the rows live in, themetricindex to walk, and the start key from(database, table, topk, entity, dimensionValues). I kept the rank table out of the request onpurpose: a service that renames it should not have to tell its callers.
{ "entity": "user1", "dimensionValues": { "category": "fruit" }, "limit": 10 }{ "topks": [ { "value": "item9", "metric": 42, "properties": { "brand": "acme" } }, { "value": "item3", "metric": 17, "properties": { "brand": "acme" } } ], "count": 2 }When the declaration is global,
entityin the request is ignored — a global ranking is one row for everyone, so there is nothing to narrow. An unknowntopkis a 400 naming it.I put the read on
POST, notGET. A ranking is picked by naming values, and a query string cannot carry those onto an immutable request. The downside is that a read stops looking like one to anything in front of the server — no caching, andReadOnlyRequestFilterhas to know about it by hand. It now matches on a path segment (/topks/)as well as on a suffix, since this path ends in a variable. If the exception list grows, the filter should read the annotation instead of a name list.
Dimension values are positional against the declaration, and a value the request omits reads back as an empty string rather than as an error. So a partially specified request quietly reads the ranking split on empty values instead of the one the caller meant. I'll leave that validation for a follow-up.
Stacked on #496 — that needs to land first.
Part of #386.
Test plan
./gradlew :engine:test --tests '*RankScanTest*'— rank table resolution, key composition, global entity, unknowntopk./gradlew :server:test --tests '*MetadataAggQueryControllerE2ETest*'— the endpoint over HTTP, per-entity and global rankings, dimension splits, paging./gradlew :server:test --tests '*ReadOnlyRequestFilterTest*'— the read passes in read-only mode./gradlew spotlessCheck build— formatting and full buildAI Assistance